-
Notifications
You must be signed in to change notification settings - Fork 203
[IRN-6178] [BpkCardList] Enable Physical Scroll For Row #3974
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
[IRN-6178] [BpkCardList] Enable Physical Scroll For Row #3974
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request enables physical scroll for row layouts in the BpkCardList component by modifying the carousel behavior to support native scrolling on all screen sizes instead of just mobile. The changes address an issue where programmatic navigation and manual scrolling could conflict, causing unwanted index reverts.
- Fixed scroll lock mechanism and intersection observer conflicts during programmatic navigation
- Enabled horizontal scrolling for all screen sizes instead of mobile-only
- Added programmatic navigation state tracking to prevent visibility-based index updates during scrolling
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| BpkCardListCarousel.tsx | Added programmatic navigation tracking logic and fixed useEffect dependency arrays to prevent scroll conflicts |
| BpkCardListCarousel.module.scss | Moved overflow-x: scroll from mobile-only to all screen sizes and reorganized scroll-related CSS properties |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if (programmaticTargetIndexRef.current === currentIndex) { | ||
| programmaticTargetIndexRef.current = null; | ||
| } | ||
| } else if (programmaticTargetIndexRef.current == null) { |
Copilot
AI
Sep 27, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use strict equality (===) instead of loose equality (==) for null comparison to follow TypeScript best practices.
| } else if (programmaticTargetIndexRef.current == null) { | |
| } else if (programmaticTargetIndexRef.current === null) { |
|
Visit https://backpack.github.io/storybook-prs/3974 to see this build running in a browser. |
| clearTimeout(openSetStateLockTimeoutRef.current); | ||
| } | ||
| }; | ||
| }, [root]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this necessary Alex (@rabisse) ?
|
Visit https://backpack.github.io/storybook-prs/3974 to see this build running in a browser. |
…hub.com:Skyscanner/backpack into IRN-6178_BpkCardList_EnablePhysicalScrollForRow
|
Visit https://backpack.github.io/storybook-prs/3974 to see this build running in a browser. |
1 similar comment
|
Visit https://backpack.github.io/storybook-prs/3974 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3974 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3974 to see this build running in a browser. |
Remember to include the following changes:
[Clover-123][BpkButton] Updating the colourREADME.md(If you have created a new component)README.md